ddl : support ADMIN REPAIR TABLE to override bad tableInfo in meta & supply a REPAIR MODE for safely restart. #12046

AilinKid · 2019-09-05T09:08:52Z

What problem does this PR solve?

This need the parser #547 merged first.

In some corner cases or encountering serious bugs, DDL operations update table structure error occurred, causing TiDB panic and failing to restart the service.

Admin Repair Table t Create Table Statement.

create new tableInfo from create table statement to override bad tableInfo in meta.

What is changed and how it works?

There are 2 steps to fix this:
1 : support special mode to start TiDB
2 : support a way to fix the bad tableInfo meta in KV store

Check List

Tests

Unit test
Integration test

Code changes

Has exported function/method change
Has exported variable/fields change
Has interface methods change

Related changes

Need to update the documentation

Release note

support ADMIN REPAIR TABLE to override bad tableInfo in meta & supply a REPAIR MODE for safely restart..

ddl/ddl_api.go

kennytm · 2019-10-21T03:34:10Z

What would happen with partitioned tables?

AilinKid · 2019-10-21T03:51:44Z

What would happen with partitioned tables?

By now, partitioned tables are not supported to repair in this version. I will focus on it!

codecov · 2019-10-22T11:20:46Z

Codecov Report

❗ No coverage uploaded for pull request base (master@5a589c9). Click here to learn what that means.
The diff coverage is 74.6478%.

@@             Coverage Diff             @@
##             master     #12046   +/-   ##
===========================================
  Coverage          ?   80.1314%           
===========================================
  Files             ?        475           
  Lines             ?     117477           
  Branches          ?          0           
===========================================
  Hits              ?      94136           
  Misses            ?      15888           
  Partials          ?       7453

zimulala · 2019-10-24T02:09:00Z

Related Design Doc : fix broken table

It's the internal docs and we'd better remove it.

ddl/ddl_api.go

domain/domain.go

config/config.go

config/config.toml.example

ddl/ddl_api.go

util/admin/admin.go

planner/core/preprocess.go

util/admin/admin.go

ddl/ddl_api.go

zimulala · 2019-11-01T10:04:28Z

domain/domain.go

+						// clean the tables and set repaired table.
+						repairedDB.Tables = []*model.TableInfo{}
+						repairedDB.Tables = append(repairedDB.Tables, tbl)
+						RepairDBInfoMap[di.ID] = repairedDB


I think we need to check this map when executing create table. The scenario is as follows:
There is a DDL operation that creates the table dbA_tblB before the operation of repairing dbA_tblB is started.

Make sense, I will check this.

djshow832

Any test cases?

planner/core/preprocess.go

djshow832 · 2019-11-08T09:09:48Z

planner/core/preprocess.go

@@ -724,6 +762,36 @@ func (p *preprocessor) handleTableName(tn *ast.TableName) {
 	tn.DBInfo = dbInfo
 }

+func (p *preprocessor) handleRepairName(tn *ast.TableName) {
+	dbMap := domain.GetDomain(p.ctx).GetTablesInRepair()
+	// tn only has the Schema here.


I don't understand the comment.

tn is a variable, addressed.

zimulala

Do we need to consider how to cancel this job?

AilinKid · 2019-12-02T08:22:24Z

/run-all-tests

bb7133 · 2019-12-03T01:45:30Z

tidb-server/main.go

+		repairString = repairString[1 : len(repairString)-1]
+	}
+	return strings.FieldsFunc(repairString, func(r rune) bool {
+		if r == ',' || r == ' ' || r == '"' {


How about return if r == ',' || r == ' ' || r == '"'?

bb7133 · 2019-12-03T01:49:35Z

util/admin/admin.go

@@ -104,7 +104,7 @@ func IsJobRollbackable(job *model.Job) bool {
 		model.ActionTruncateTable, model.ActionAddForeignKey,
 		model.ActionDropForeignKey, model.ActionRenameTable,
 		model.ActionModifyTableCharsetAndCollate, model.ActionTruncateTablePartition,
-		model.ActionModifySchemaCharsetAndCollate:
+		model.ActionModifySchemaCharsetAndCollate, model.ActionRepairTable:


Please add a test if we support canceling the job.

AilinKid · 2019-12-03T02:32:57Z

/run-all-tests

zimulala · 2019-12-03T02:39:54Z

ddl/ddl_api.go

+			return ErrRepairTableFail.GenWithStackByArgs("Column " + newOne.Name.L + " has lost")
+		}
+		if newOne.Tp != old.Tp {
+			return ErrRepairTableFail.GenWithStackByArgs("Column " + newOne.Name.L + " type should be the same")


Could we add the type of column to the error message?
Do we need to check the Flen ?

We don't assume the too much element in the old (broken) version is reasonable，providing a certain degree of freedom for more robust repairs.

zimulala

LGTM

AilinKid · 2019-12-03T05:47:13Z

/run-all-tests

…supply a REPAIR MODE for safely restart. (pingcap#12046)

AilinKid changed the title ~~ddl : supprot admin repair table to~~ ddl : support ADMIN REPAIR TABLE to override bad tableInfo in meta & supply a REPAIR MODE for safely restart. Sep 5, 2019

AilinKid changed the title ~~ddl : support ADMIN REPAIR TABLE to override bad tableInfo in meta & supply a REPAIR MODE for safely restart.~~ ddl : support ADMIN REPAIR TABLE to override bad tableInfo in meta & supply a REPAIR MODE for safely restart.[DNM] Sep 5, 2019

AilinKid added the status/DNM label Sep 5, 2019

AilinKid mentioned this pull request Sep 5, 2019

parser : support admin repair table syntax pingcap/parser#547

Merged

kennytm reviewed Oct 21, 2019

View reviewed changes

ddl/ddl_api.go Outdated Show resolved Hide resolved

AilinKid force-pushed the admin_repair_table branch from 67570dd to 7844a73 Compare October 22, 2019 11:20

AilinKid added component/DDL-need-LGT3 and removed status/DNM labels Oct 22, 2019

AilinKid changed the title ~~ddl : support ADMIN REPAIR TABLE to override bad tableInfo in meta & supply a REPAIR MODE for safely restart.[DNM]~~ ddl : support ADMIN REPAIR TABLE to override bad tableInfo in meta & supply a REPAIR MODE for safely restart. Oct 22, 2019

AilinKid requested review from kennytm, crazycs520, zimulala, foreyes and bb7133 and removed request for foreyes October 23, 2019 02:43

AilinKid requested a review from djshow832 October 28, 2019 03:18

AilinKid mentioned this pull request Oct 29, 2019

*: support the TiFlash replica of table #12453

Merged

crazycs520 reviewed Nov 1, 2019

View reviewed changes

ddl/ddl_api.go Outdated Show resolved Hide resolved

crazycs520 reviewed Nov 1, 2019

View reviewed changes

domain/domain.go Outdated Show resolved Hide resolved

zimulala reviewed Nov 1, 2019

View reviewed changes

ddl/ddl_api.go Outdated Show resolved Hide resolved

zimulala added the status/DNM label Nov 1, 2019

zimulala reviewed Nov 1, 2019

View reviewed changes

djshow832 reviewed Nov 8, 2019

View reviewed changes

AilinKid force-pushed the admin_repair_table branch from 5d51ca1 to 65110bc Compare November 11, 2019 03:36

zimulala reviewed Dec 2, 2019

View reviewed changes

AilinKid and others added 2 commits December 2, 2019 16:15

add rollback/cancel support

2365dbf

Merge branch 'master' into admin_repair_table

9de3312

AilinKid added the status/all tests passed label Dec 2, 2019

Merge branch 'master' into admin_repair_table

ccc4d34

bb7133 reviewed Dec 3, 2019

View reviewed changes

AilinKid and others added 2 commits December 3, 2019 10:21

address comment

a03e642

Merge branch 'master' into admin_repair_table

1941108

zimulala reviewed Dec 3, 2019

View reviewed changes

address comment

86b202c

zimulala approved these changes Dec 3, 2019

View reviewed changes

Merge branch 'master' into admin_repair_table

d2e578d

AilinKid requested a review from a team as a code owner December 3, 2019 05:40

ghost requested review from eurekaka and alivxxx and removed request for a team December 3, 2019 05:40

eurekaka removed their request for review December 3, 2019 05:41

AilinKid removed the request for review from alivxxx December 3, 2019 05:43

Merge branch 'master' into admin_repair_table

00cd67a

AilinKid added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Dec 3, 2019

AilinKid merged commit 97a4fae into pingcap:master Dec 3, 2019

XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019

ddl : support ADMIN REPAIR TABLE to override bad tableInfo in meta & …

6eff6b7

…supply a REPAIR MODE for safely restart. (pingcap#12046)

AilinKid mentioned this pull request Feb 3, 2020

dev: add admin repair table statement. pingcap/docs-cn#2205

Merged

12 tasks

you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl : support ADMIN REPAIR TABLE to override bad tableInfo in meta & supply a REPAIR MODE for safely restart. #12046

ddl : support ADMIN REPAIR TABLE to override bad tableInfo in meta & supply a REPAIR MODE for safely restart. #12046

AilinKid commented Sep 5, 2019 •

edited

Loading

kennytm commented Oct 21, 2019

AilinKid commented Oct 21, 2019

codecov bot commented Oct 22, 2019 •

edited

Loading

zimulala commented Oct 24, 2019 •

edited

Loading

zimulala Nov 1, 2019

AilinKid Nov 8, 2019

djshow832 left a comment

djshow832 Nov 8, 2019

AilinKid Nov 11, 2019

zimulala left a comment

AilinKid commented Dec 2, 2019

bb7133 Dec 3, 2019

AilinKid Dec 3, 2019

bb7133 Dec 3, 2019

AilinKid Dec 3, 2019 •

edited

Loading

AilinKid commented Dec 3, 2019

zimulala Dec 3, 2019

AilinKid Dec 3, 2019 •

edited

Loading

AilinKid Dec 3, 2019

zimulala left a comment

AilinKid commented Dec 3, 2019

ddl : support ADMIN REPAIR TABLE to override bad tableInfo in meta & supply a REPAIR MODE for safely restart. #12046

ddl : support ADMIN REPAIR TABLE to override bad tableInfo in meta & supply a REPAIR MODE for safely restart. #12046

Conversation

AilinKid commented Sep 5, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

kennytm commented Oct 21, 2019

AilinKid commented Oct 21, 2019

codecov bot commented Oct 22, 2019 • edited Loading

Codecov Report

zimulala commented Oct 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djshow832 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zimulala left a comment

Choose a reason for hiding this comment

AilinKid commented Dec 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AilinKid Dec 3, 2019 • edited Loading

Choose a reason for hiding this comment

AilinKid commented Dec 3, 2019

Choose a reason for hiding this comment

AilinKid Dec 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zimulala left a comment

Choose a reason for hiding this comment

AilinKid commented Dec 3, 2019

AilinKid commented Sep 5, 2019 •

edited

Loading

codecov bot commented Oct 22, 2019 •

edited

Loading

zimulala commented Oct 24, 2019 •

edited

Loading

AilinKid Dec 3, 2019 •

edited

Loading

AilinKid Dec 3, 2019 •

edited

Loading